Block Switcher: Render disabled button even if multi-selection #13431
Conversation
aduth
requested review from
gziolo,
jasmussen,
chrisvanpatten and
jorgefilipecosta
|
I dig it, thanks for the work. I think it is correct to use a disabled-looking (because it is disabled) icon in the switcher, when no transformation is necessary. Is it maybe an opportunity to explain what is going on — can a disabled button show a tooltip? Like if you hovered the disabled button, could it say "No transformations available", or something to that effect? GIF:
It really would be nice if we could swap out that "first of type" icon with something else. Perhaps this?
Also, this made me realize (which is completely unrelated to this branch and should not be fixed here) that we are scaling down the block icons! ┻━┻ ︵ヽ(`Д´)ﾉ︵ ┻━┻
The material icons are 24x24, but it's scaled down to 20x20. Which makes them blurry on the majority of screens in the world:
When/how did this regress? I feel like we just fixed that recently. I think we might need an e2e test for icons :D |
|
@jasmussen those pesky icons! Might be worth looking into screenshot tests for some of the UI stuff that regresses regularly. I like the idea of a separate icon for multi-block switching that spans multiple block types, but I'm also not too bothered by the first-block-type icon if it's radically easier. I think the benefits here outweigh the costs. |
There was a problem hiding this comment.
Let's do it 👍
Is it maybe an opportunity to explain what is going on — can a disabled button show a tooltip? Like if you hovered the disabled button, could it say "No transformations available", or something to that effect?
We don't do it also for blocks which don't offer transformations. I think it should be tackled separately.
Another thing we could do in its own PR, which I raised in the past, is Edit as HTML mode:
It could also render the disabled icon for consistency.
aduth
force-pushed
the
update/block-switcher-render-multi-block
branch
from
e562007
to
1c52dc1
Compare
|
I've pushed some updates per feedback, specifically using a different icon when blocks in a selection are not of same type. Same type, with transforms available (no change from master):
Different type, no transforms available (master would have shown nothing):
Same type, no transforms available (master would have shown nothing):
|
|
Digging that. I'm sure we might keep at the icon discussions, but for now I think this is lovely. Thanks. |
aduth
force-pushed
the
update/block-switcher-render-multi-block
branch
from
1c52dc1
to
2aaf4c2
Compare
…rnmobile/372-enter-key-detection-to-title * 'master' of https://github.com/WordPress/gutenberg: (29 commits) Update for RangeControl documentation (#12564) Plugin: Deprecate gutenberg_load_list_reusable_blocks (#13456) Update the columns attribute in onSelectImages so that if images are removed via the media modal, the columns can't be higher than the new number of images (#13488) Replace the fullscreen "exit" icon with a back arrow (#13403) Include :visited links in button color (#12183) Amazon Kindle block (#13510) Plugin: Deprecate gutenberg_prepare_blocks_for_js (#13457) Add watcher on Linux: change fs to node-watch (#13448) Plugin: Deprecate `gutenberg` theme support (#13458) Datepicker: Add inValidDay support (#12962) Block Switcher: Render disabled button even if multi-selection (#13431) Plugin: Deprecate gutenberg_register_post_types (#13468) Plugin: Deprecate register_tinymce_scripts (#13466) Set minimum of words for RSS excerpt (#13502) Plugin: Deprecate gutenberg_get_block_categories (#13454) Plugin: Deprecate gutenberg_content_block_version (#13469) API Fetch: Expose nonce on created middleware function (#13451) Plugin: Remove list screens integrations (#13459) Plugin: Remove core-defined block detection functions (#13467) Spec Parser: Move generated spec parser to package (#13493) ...
Previously: #11600 (comment)
This pull request seeks to remove a condition present in the
BlockSwitchercomponent which causes it to render nothing if there are no possible transformations and there is a multi-selection of blocks.Without these changes, there is an inconsistency based on an assumption that transformations can only apply to a set of blocks of the same type. While currently true, there is not a guarantee that this will always be the case, and the component itself otherwise makes no expectation that it would be (if transformations exist, a dropdown will be shown with the icon of the first block, regardless of whether all blocks in the selection are of the same type). Additionally, the current behavior would cause nothing to be rendered even if the multi-selection was of blocks of the same type, if there were no multi-transforms for multiple blocks of that type. This is inconsistent with both multi-selections of the same type where a transformation does exist (dropdown shown with icon of that block type), and singular selections of a type where no transformation exists (dropdown not shown, but disabled icon shown of that block type).
Alternatives:
This does present an awkward scenario where multiple blocks of different types shows an icon of the first type:
Instead, we may consider:
Testing instructions:
Verify that the BlockSwitcher is shown always, regardless of the blocks composing the selection. The icon should correspond to the first block in the selection, and should be disabled if and only if there are neither valid transforms nor applicable styles to choose from.